Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ignore encryption preferences #6612

Merged
merged 2 commits into from
Mar 12, 2025

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented Mar 3, 2025

Always prefer encryption if it is available.

This is in preparation for PGP-contacts as we want to clearly separate encrypted and unencrypted chats instead of having them switch between encrypted and unencrypted based on majority vote.

@link2xt link2xt force-pushed the link2xt/ignore-encryption-preferences branch 2 times, most recently from 7ad7fcc to c4d8826 Compare March 9, 2025 22:40
@link2xt link2xt changed the base branch from main to link2xt/clone-accounts March 9, 2025 22:40
@link2xt link2xt force-pushed the link2xt/ignore-encryption-preferences branch 3 times, most recently from c8b1f04 to f2e3f44 Compare March 10, 2025 00:41
@link2xt link2xt changed the base branch from link2xt/clone-accounts to link2xt/account-apis March 10, 2025 00:43
@link2xt link2xt force-pushed the link2xt/ignore-encryption-preferences branch from f2e3f44 to f92606b Compare March 10, 2025 00:45
@link2xt link2xt marked this pull request as ready for review March 10, 2025 01:51
@link2xt link2xt marked this pull request as draft March 10, 2025 01:55
@link2xt link2xt force-pushed the link2xt/account-apis branch from 1e31f3e to aba9544 Compare March 10, 2025 22:45
@link2xt link2xt force-pushed the link2xt/ignore-encryption-preferences branch from 723e691 to 40e477e Compare March 10, 2025 22:45
@link2xt link2xt force-pushed the link2xt/account-apis branch from aba9544 to 491d6ab Compare March 10, 2025 23:55
@link2xt link2xt force-pushed the link2xt/ignore-encryption-preferences branch 4 times, most recently from 91d5151 to 69c2277 Compare March 11, 2025 01:32
link2xt added 2 commits March 11, 2025 03:54
Encryption preference is sent in Autocrypt header,
but otherwise ignored.

Delta Chat always prefers encryption if it is available.
@link2xt link2xt force-pushed the link2xt/ignore-encryption-preferences branch from 69c2277 to 859b5b8 Compare March 11, 2025 03:54
@link2xt link2xt changed the base branch from link2xt/account-apis to main March 11, 2025 18:19
@link2xt link2xt marked this pull request as ready for review March 11, 2025 18:19
@link2xt link2xt requested review from Hocuri and iequidoo March 11, 2025 18:20
@link2xt
Copy link
Collaborator Author

link2xt commented Mar 11, 2025

We need to think about hermes.radio usecase, it sets the config but the config is not going to have any effect. hermes.radio currently relies on stripping Autocrypt to downgrade everyone to unencrypted. Maybe they don't want to have encryption key at all, or maybe could be fine with encryption if we build high compression mode into the core so the server does not need to recompress. I am not removing the setting completely because we have it set in provider database for hermes.radio, but we are definitely breaking this usecase so need to discuss it separately.

Copy link
Collaborator

@iequidoo iequidoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically this is "ignore own encryption preference", peer's encryption preference is already ignored currently, so maybe clarify this in the comment message?

Copy link
Collaborator

@Hocuri Hocuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this break the hermes.radio usecase? If they strip the Autocrypt headers, then DC can't encrypt, so it won't.

What might break their usecase is the new QR codes that will be encrypted with a symmetric secret and don't leak Autocrypt header, but we can think of this when it's time (I mean, for now they are only a rough idea, nothing concrete).

If this makes problems for them, we can always hardcode non-encryption (e.g. add if self.get_config(Config::E2eeEnabled).await? { return Ok(false); } to the beginning of should_encrypt()).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FTR, the important semantic change is in this file here, everything else is just tests

@link2xt link2xt merged commit 51bbdad into main Mar 12, 2025
66 checks passed
@link2xt link2xt deleted the link2xt/ignore-encryption-preferences branch March 12, 2025 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants